520704 - Build problems with libffi and PGO

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Mark Finkle (:mfinkle) (use needinfo?)]

A comment on Mozillazine Forums seems to indicate that libffi may not compile with PGO enabled builds:

http://forums.mozillazine.org/viewtopic.php?f=42&t=1452475&p=7670785#p7670785

Comment 1

[Mark Finkle (:mfinkle) (use needinfo?)]

I'm not sure we support PGO on Linux, do we?

Comment 2

[benisty.e]

Although this bug has been reported on OS X platform, I can confirm this on Linux as well (GCC 4.4.1). Build fails with:

make[3]: Entering directory
`/home/eb/pkg/firefox-hg-192/src/meh/js/ctypes/libffi'
make[3]: *** No rule to make target `clobber_all'.  Stop.
make[3]: Leaving directory
`/home/eb/pkg/firefox-hg-192/src/meh/js/ctypes/libffi'
make[2]: *** [clobber_all] Error 2
make[2]: Leaving directory `/home/eb/pkg/firefox-hg-192/src/meh'
make[1]: *** [maybe_clobber_profiledbuild] Error 2
make[1]: Leaving directory `/home/eb/pkg/firefox-hg-192/src'
make: *** [profiledbuild] Error 2

This bug has been introduced by commit 8dcb04076a94: Land jsctypes. b=513783, r=jorendorff, sr=bsmedberg, a=bsmedberg
http://hg.mozilla.org/releases/mozilla-1.9.2/log?rev=8dcb04076a94

> I'm not sure we support PGO on Linux, do we?
I don't know if it's officially supported but it has been working since ages with significant improvements over non-profiled builds.

Comment 3

[[e-mail]]

A fix for bug 520823 would definitely help here, but we could probably hack something up to make PGO use a saner clean target.

Comment 4

[(not currently active) Ted Mielczarek]

(In reply to comment #1)
> I'm not sure we support PGO on Linux, do we?

We like to keep it working.

Also, the use of the "clobber_all" target is just historical, AFAICT. I shuffled some things around, but kept the original make target there.

Comment 5

[koralik]

Simple fix for that posted on Archlinux forum by jleach:
> To get it to build I just removed 'clobber_all' from the line 229 of mozilla-1.9.2/Makefile.in
> 
> Code:
> 
> -maybe_clobber_profiledbuild: clobber_all
> +maybe_clobber_profiledbuild:

Comment 6

[(not currently active) Ted Mielczarek]

That's not a good fix. I don't see how a PGO build could possibly work with that. With GCC, you need to recompile on each pass, which is why the clobber is there.

Comment 7

[Bill Lancashire]

My temporary fix ( for LINUX) is to edit line 233 of 'client.mk' as follows:

- $(MAKE) -f $(TOPSRCDIR)/client.mk maybe_clobber_profiledbuild
+ $(MAKE) -f $(TOPSRCDIR)/client.mk clobber_all

After applying this patch the 2 PGO builds progress as expected. I don't understand why the 'libffi' directory doesn't like 'maybe_clobber_profiledbuild'. I guess it's an issue with 'makefile.in' but I don't know enough about 'make' to figure it out myself.

Comment 8

[Bill Lancashire]

I really don't think this problem compiling a 'PGO' build in Linux has anything actually to do with 'libffi' and PGI.  I carried out the following little trial:

1.) un-tar the Firefox source from https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/3.6b3-candidates/build1/source/

2.)from the 'mozilla' directory, carry out commands as follows:

gmake -f client.mk distclean ---- result: process completes a expected;

gmake -f client.mk clean ---- result: process completes a expected;

gmake -f client.mk clobber ---- result: process completes a expected;

gmake -f client.mk clobber_all ---- result: process completes a expected;

gmake -f client.mk maybe_clobber_profiledbuild
 ---- result: process aborts exactly as during a PGO build with the following:

make[5]: Leaving directory `/usr/src/firefox-3.6b3.source/mozilla/ffobjs/ff-pgo/browser/app/profile/extensions'
make[4]: Leaving directory `/usr/src/firefox-3.6b3.source/mozilla/ffobjs/ff-pgo/browser/app'
make[3]: Leaving directory `/usr/src/firefox-3.6b3.source/mozilla/ffobjs/ff-pgo/browser'
make[3]: Entering directory `/usr/src/firefox-3.6b3.source/mozilla/ffobjs/ff-pgo/js/ctypes/libffi'
make[3]: *** No rule to make target `clobber_all'. Stop

So to me it seems that there is an issue with one or more 'makefiles' in the source or something related in the configuration.

I do not has the knowledge to be able to find the cause myself, but it must be trivial for someone who can.

Hopefully could be looked at.

Comment 9

[(not currently active) Ted Mielczarek]

(In reply to comment #7)
> My temporary fix ( for LINUX) is to edit line 233 of 'client.mk' as follows:
> 
> - $(MAKE) -f $(TOPSRCDIR)/client.mk maybe_clobber_profiledbuild
> + $(MAKE) -f $(TOPSRCDIR)/client.mk clobber_all

We can't take this patch either, since on Windows we don't want to clobber in between passes (which is why the maybe_clobber_profiledbuild target exists).

You could try just changing the line here:
http://mxr.mozilla.org/mozilla-central/source/Makefile.in#210

from clobber_all to clean.

Comment 10

[Bill Lancashire]

(In reply to comment #9)
> (In reply to comment #7)
> > My temporary fix ( for LINUX) is to edit line 233 of 'client.mk' as follows:
> > 
> > - $(MAKE) -f $(TOPSRCDIR)/client.mk maybe_clobber_profiledbuild
> > + $(MAKE) -f $(TOPSRCDIR)/client.mk clobber_all
> 
> We can't take this patch either, since on Windows we don't want to clobber in
> between passes (which is why the maybe_clobber_profiledbuild target exists).
> 
> You could try just changing the line here:
> http://mxr.mozilla.org/mozilla-central/source/Makefile.in#210
> 
> from clobber_all to clean.

I have tried changing 'clobber_all' to 'clean' as you suggested here:
http://mxr.mozilla.org/mozilla-central/source/Makefile.in#210

and now the build PGO build process progresses as expected on Linux.

I can make a suitable patch for my own use, but I am not confident enough to make one in the format necessary to submit here in bugzilla.

Would someone care to do this if the change would be a 'good one?.

Comment 11

[(not currently active) Ted Mielczarek]

All you need to do is make the change, run "hg diff", and attach the resulting output here using the "add an attachment" link. If you can do that, I'll review and land it for you.

Comment 12

[Bill Lancashire]

Attachment: Patch 'clobber_all' on Linux

I have tried to make the patch.  My first time so someone please check.

Comment 13

[(not currently active) Ted Mielczarek]

Comment on attachment 414496 [details] [diff] [review]
Patch 'clobber_all' on Linux

Looks good, thanks!

Comment 14

[Benjamin Smedberg]

a=me for immediate checkin on mozilla-central restricted tree, if that's necessary.

Comment 15

[Bill Lancashire]

I'm really not familiar at all with the process on 'buzilla'.  Can I now assume that someone will check this in if deemed appropriate?.

Comment 16

[(not currently active) Ted Mielczarek]

Yeah, the "checkin-needed" keyword indicates that there's a patch here that's ready for landing, and there are people who often look for bugs with that keyword to land. (Our main development tree is frozen right now for 3.6RC, so it won't be right away, but soon.)

Comment 17

[Reed Loden [:reed]]

http://hg.mozilla.org/mozilla-central/rev/5ff5350270a7

Comment 18

[Reed Loden [:reed]]

Comment on attachment 414496 [details] [diff] [review]
Patch 'clobber_all' on Linux

Requesting approval1.9.2 for this build fix.

Comment 19

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 414496 [details] [diff] [review]
Patch 'clobber_all' on Linux

a192=beltzner, reed has been scolded for landing this without the approval and has apologized sufficiently.

Comment 20

[Reed Loden [:reed]]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/36709247c790

Comment 21

[archlinux]

After building both 3.6b4 and 3.7a1pre with this patch I now have a problem with firefox not closing properly:

https://bugzilla.mozilla.org/show_bug.cgi?id=533632

Comment 22

[Bill Lancashire]

I don't see any problem with my PGO builds Firefox 3.6b4 and Firefox 3.6b5(build 1).

I can only suggest that the problem you are experiencing is due to the fact that your build is now actually a PGO build when this patch is used.  PGO can be problematic in Linux builds as discussed in various posts and may need 'debugging' in individual cases.  I guess it is for this reason that PGO has not yet been 'turned-on' in official Mozilla Firefox builds for Linux.